Repository navigation
Assorted bug fixes - #263
nicoweidner wants to merge 15 commits into
Conversation
1. yaml creation missing encoding. 2. NoAssert cannot be written to a file (not serializable) as it does not have default repr. 3. Problem creating relationships Signed-off-by: OzTamari <oz.tamari@whitesourcesoftware.com>
Signed-off-by: OzTamari <oz.tamari@whitesourcesoftware.com>
Signed-off-by: OzTamari <oz.tamari@whitesourcesoftware.com>
1. yaml creation missing encoding. 2. NoAssert cannot be written to a file (not serializable) as it does not have default repr. 3. Problem creating relationships Signed-off-by: OzTamari <oz.tamari@whitesourcesoftware.com>
Signed-off-by: OzTamari <oz.tamari@whitesourcesoftware.com>
Signed-off-by: rammatzkvosky <ram.matzkovsky@whitesourcesoftware.com>
…ue is not available at WhiteSource Updating setup.py version to '0.7.0a3.post6' Signed-off-by: rammatzkvosky <ram.matzkovsky@whitesourcesoftware.com>
Signed-off-by: Nicolaus Weidner <nicolaus.weidner@tngtech.com>
meretp
left a comment
meretp
left a comment
There was a problem hiding this comment.
Thanks for continuing this PR!
I have a few minor comments, but I'm not sure if the rebase will take care of some of it. I haven't looked at the changes to the example files yet, because I think that will change more with the rebase.
| @@ -112,3 +118,5 @@ def validate_relationship(self, messages): | |||
| "Relationship type must be one of the constants defined in " | |||
| "class spdx.relationship.Relationship" | |||
| ) | |||
There was a problem hiding this comment.
Why do we use these two functions if the validate method only calls the validate_relationship method? IMHO we should only use the validate_relationship method.
| package_objects.append({"Package": package_info_object}) | ||
| # create a list of the packages' ids for the 'documentDescribes' field | ||
| package_spdx_id = package_info_object.get("SPDXID") | ||
| document_describes_objects.append(package_spdx_id) |
There was a problem hiding this comment.
With this we will add all packages at document level to the list of documentDescribes SPDXIDs. As far as I understood only the packages which are part of a DESCRIBES-Relationship should be listed here. I am not sure if the rebase will tackle this already.
| # Add relationship | ||
| for relate_node in self.relationships(): | ||
| relate_triple = (doc_node, self.spdx_namespace.relationship, relate_node) | ||
| self.graph.add(relate_triple) | ||
|
|
There was a problem hiding this comment.
I did the same as in your comment here and still got the same result. I think this should be fixed before merging this PR.
Signed-off-by: Nicolaus Weidner <nicolaus.weidner@tngtech.com>
97fcb17 to
293c811
Compare
Signed-off-by: Nicolaus Weidner <nicolaus.weidner@tngtech.com>
| "ns1:relationship": {"ns1:Relationship": {"@rdf:about": "SPDXRef-DOCUMENT", | ||
| "ns1:relatedSpdxElement": "SPDXRef-Package", | ||
| "ns1:relationshipType": "DESCRIBES"}}, |
There was a problem hiding this comment.
A comment about these additions:
This structure looks reasonable to me and is apparently what our rdf writer produces at the moment. It may not be what the spec intends, though:
The structure in e.g. this example is different: The related element is usually expanded (making the whole thing completely unreadable), and there doesn't seem to be a reference to the current element (maybe relationships are put on the originating object instead of the top-level document?).
Considering that there are other oddities in this test file as well (e.g. referencesFile and describesPackage doesn't exist in example files from the spec or java tools), and the documentation on the desired state is unclear, I'd leave it at this for now. I will ask on the ML what's going on with rdf...
There was a problem hiding this comment.
As far as I know, elements in rdf files are normally expanded at the first place they appear (which does get quite muddy, yeah).
|
Continued in #270. |
Taken over from #207.
There were some conflicts when rebasing on
main. I tried to make minimal changes, only fixing code that was obviously wrong after resolving conflicts. I will add additional changes in separate commits.NOTE: Since signoffs of original authors are missing, I will redo this PR from scratch.